Skip to content

Cloud Team Collections (S3 + Supabase) - #8052

Draft
JohnThomson wants to merge 9 commits into
masterfrom
cloud-tc-for-review
Draft

Cloud Team Collections (S3 + Supabase)#8052
JohnThomson wants to merge 9 commits into
masterfrom
cloud-tc-for-review

Conversation

@JohnThomson

@JohnThomson JohnThomson commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

The full Cloud Team Collections feature — a cloud backend (one S3 bucket + Supabase Postgres/edge functions) for Bloom's Team Collections, alongside the existing folder implementation — packaged as 9 dependency-ordered, review-grained commits. Replaces draft PR #8048 (same tree, byte-identical — verified by an empty git diff against the cloud-collections working branch), whose ~200 orchestration-grained commits were not reviewable.

Suggested review order = commit order:

  1. Design docs, wire contracts (CONTRACTS.md v1.5), project records
  2. Server: tc schema, RLS, RPCs, pgTAP (65 green)
  3. Server: edge functions for the two-phase checkin/download protocols (deno tests 33 green)
  4. Local dev stack (Supabase + MinIO; zero external infra)
  5. C# client core: auth (Firebase Option A + dev GoTrue), API client, repo cache, S3 transfer (AWSSDK v4)
  6. C# backend: CloudTeamCollection, polling monitor, progressive join, background downloads, account-switch takeover with per-collection-copy seats
  7. HTTP API layer: sharing/membership/capabilities/join cards
  8. React UI + XLF strings (en)
  9. Playwright-over-CDP E2E harness + 10 two-instance scenarios

Review caveat: each commit is a coherent reviewable unit and the order makes most of them compile, but only the FINAL tree is test-verified (C# cloud filter 428/428, full suite green, pgTAP 65/65, deno 33/33, E2E matrix green with every scenario passing on this tree). This is the accepted trade-off; per-commit CI-green is not a goal.

Bot review history (on #8048): Greptile reviewed the full diff via the file-limit bypass, found 1 P1 (S3 credential scoping) + 2 P2s — all fixed and verified resolved by its re-review ('No new blocking issues'). Devin cannot analyze a diff this size. CI green.

Remaining before merge: John's two [HUMAN] visual checks (centered check-in dialog; real web upload/download per GOING-LIVE.md 4.3) and the OUTSTANDING BUGS #0 follow-up decision (server-side same-user cross-seat check-in) — tracked in Design/CloudTeamCollections/orchestration/DOGFOOD-BATCH-1.md.

🤖 Generated with Claude Code

Devin review


This change is Reviewable

JohnThomson added a commit that referenced this pull request Jul 11, 2026
…mits, byte-identical), #8048 closed

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@JohnThomson
JohnThomson force-pushed the cloud-tc-for-review branch from 02e5610 to 8ac48df Compare July 14, 2026 14:38
@JohnThomson

Copy link
Copy Markdown
Contributor Author

@greptile-apps review

(Claude Fable 5: regenerated packaging branch — same 9 review-grained commits, now including the 13-14 Jul dogfood fixes: member display names + CONTRACTS v1.6, identity-first book resolution, --attended launch flag, remote-rename dedup, pending-invite constraint fix. Byte-identical to cloud-collections at f5a00c1.)

JohnThomson added a commit that referenced this pull request Jul 14, 2026
…#8052 head 8ac48df), bot gauntlet triggered; handoff list for John while agent is paused
@greptile-apps

greptile-apps Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This regenerated PR (byte-identical to cloud-collections) adds a documentation-and-server-hardening pass on top of the Cloud Team Collections feature. No client behavior changes; all modifications are to design docs, database migrations, and a new operational edge function.

  • Server hardening (3 new migrations): The last-admin guard trigger is replaced to serialize concurrent admin removals via FOR UPDATE on the parent tc.collections row, closing a TOCTOU race. A new tc.support_set_admin() admin-recovery RPC (service-role-only, verified by pgTAP 11b–11d) allows the Bloom team to restore admin access out-of-band. A tc.list_stale_upload_garbage() worklist RPC feeds a new sweep-stale-uploads edge function that deletes orphaned S3 object versions left by failed check-in commits.
  • Documentation additions: Design/CloudTeamCollections/SCHEMA.md adds a Mermaid ER diagram of the full tc schema. CONTRACTS.md gains a "Cloud functions" primer. The previously-flagged double-hyphen in Bloom.xlf's CollectionTab.BookNotYetDownloaded note has been replaced with a semicolon.

Important Files Changed

Filename Overview
supabase/migrations/20260717000001_tc_last_admin_guard_lock.sql Replaces the last-admin guard trigger to serialize concurrent admin removals via FOR UPDATE on the parent collection row — correctly fixes the TOCTOU race and is exercised by pgTAP test 11a.
supabase/migrations/20260717000002_tc_support_set_admin.sql Adds service-role-only admin-recovery function; correctly REVOKEs PUBLIC EXECUTE and GRANTs only to service_role; idempotent upsert logic is sound; pgTAP tests 11b–11d cover promote, insert, and auth-gate paths.
supabase/migrations/20260717000003_tc_list_stale_upload_garbage.sql Worklist RPC for the orphaned-upload sweep; SECURITY DEFINER + service_role-only grant; excludes live-transaction paths correctly; paired with pgTAP section 12.
supabase/functions/sweep-stale-uploads/index.ts New edge function for GC of orphaned S3 uploads; service-role gate is checked eagerly; correctly deletes only versions newer than the committed reference; referencedMissing counter surfaces data loss risk without destructive action.
supabase/functions/sweep-stale-uploads/index.test.ts Deno unit tests cover all four branches: normal GC, null-referenced (all orphaned), referenced-missing (skip + report), and non-service-role 403.
supabase/functions/_shared/s3.ts Adds listObjectVersions and deleteObjectVersion helpers for the sweep; no-pagination design is explicitly justified by the 7-day lifecycle rule bounding version accumulation per key.
Design/CloudTeamCollections/SCHEMA.md New ER diagram doc for the tc schema; accurate through migration 20260716000001; the 20260717 migrations don't add tables/columns so the diagram remains current.
Design/CloudTeamCollections/CONTRACTS.md Added Cloud functions section explaining the Postgres-RPC vs. edge-function distinction for reviewers; existing contract content unchanged.
DistFiles/localization/en/Bloom.xlf Adds ~15 new trans-units for cloud Team Collections UI; the previously-flagged double-hyphen in the CollectionTab.BookNotYetDownloaded note has been replaced with a semicolon.
supabase/tests/01_tc_schema_test.sql Adds pgTAP sections 11 and 12: lock-guard assertion (11a), support_set_admin promote/insert/auth-gate (11b–11d), and the stale-garbage worklist (section 12).
supabase/migrations/20260716000001_tc_get_collection_file_manifest.sql Adds get_collection_file_manifest RPC (CONTRACTS.md v1.7) — read-only, SECURITY DEFINER, granted to authenticated; exposes existing collection_group_files data for pinned downloads.
src/BloomExe/TeamCollection/Cloud/CloudAuth.cs Provider-agnostic session core; DPAPI-backed persistent store for production; proactive refresh at 80% TTL; large new file with thorough doc comments.

Reviews (7): Last reviewed commit: "Cloud TC E2E: Playwright-over-CDP harnes..." | Re-trigger Greptile

Comment thread DistFiles/localization/en/Bloom.xlf
JohnThomson added a commit that referenced this pull request Jul 14, 2026
…e P1)

L10NSharp routes <note> content through XML-comment parsing, where '--' is
illegal, so the CollectionTab.BookNotYetDownloaded note's '... confirmation --
may belong ...' crashed Bloom at startup with "An XML comment cannot contain
'--'" once localization loaded. Replace the '--' with a semicolon. Found by
Greptile on PR #8052; matches the known failure the xlf-strings skill documents.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@JohnThomson
JohnThomson force-pushed the cloud-tc-for-review branch from 8ac48df to b45e5e2 Compare July 14, 2026 23:05
@JohnThomson
JohnThomson force-pushed the cloud-tc-for-review branch from b45e5e2 to 8fbe66e Compare July 15, 2026 02:21
@JohnThomson

Copy link
Copy Markdown
Contributor Author

@greptile-apps review

(Claude Fable 5) Regenerated review branch: re-merged origin/master, and folded in fixes for the Devin slice-review findings (upload-race lock, cache LockedSeat/LocalVersionSeq, experimental-feature-flag token collision, blank cloud history-event label). Byte-identical to the cloud-collections working branch.

JohnThomson added a commit that referenced this pull request Jul 15, 2026
@JohnThomson
JohnThomson force-pushed the cloud-tc-for-review branch from 8fbe66e to aa3842f Compare July 15, 2026 15:13
@JohnThomson

Copy link
Copy Markdown
Contributor Author

@greptile-apps review

(Claude Fable 5) Regenerated after a fresh-eyes review pass over yesterday's fixes: hardened the remote-rename guard (now keyed to the authoritative repo lock, so a checked-out-and-renamed book can't be reverted/clobbered at startup), display-name cache now invalidates on account switch, and the run.sh freshness check uses build outputs that incremental builds actually touch. Regeneration scripts now live in-repo (Design/CloudTeamCollections/orchestration/regen-*.sh). Byte-identical to cloud-collections.

@JohnThomson
JohnThomson force-pushed the cloud-tc-for-review branch from aa3842f to 51da40f Compare July 15, 2026 15:27
@JohnThomson

Copy link
Copy Markdown
Contributor Author

@greptile-apps review

(Claude Fable 5) Regenerated after a documentation cleanup: removed 13 spent orchestration scratch files (one-time agent launch prompts, an implemented design sketch, stale scouting notes) — the diff shrinks from 256 to 243 files. The durable docs (design, CONTRACTS, GOING-LIVE deploy plan, task specs, state log, regeneration procedure) are unchanged. No code changes beyond three comment pointers in the e2e harness. Byte-identical to cloud-collections.

@JohnThomson
JohnThomson force-pushed the cloud-tc-for-review branch from 51da40f to 98bc352 Compare July 16, 2026 07:38
@JohnThomson

Copy link
Copy Markdown
Contributor Author

@greptile-apps review

(Claude Code, Opus 4.8) Regenerated after applying the /simplify quality-review pass across the whole branch — intended as no behavior change:

  • C#: dead-API removal + reuse consolidation (duplicate S3 client → CloudBookTransfer.BuildDefaultClient; manual S3 pagination → S3Extensions.ListAllObjects; hoisted AvailablePath reuse; localManifest hash-reuse in UploadChangedFiles with a new pinning test).
  • C# file-org (pure relocation): CloudTeamCollection.CollectionFiles.cs + TeamCollection.AutoApply.cs partials, Dev/FirebaseCloudAuthProvider split out of CloudAuth.cs, and HandleReceiveUpdates' loop → CloudTeamCollection.ReceiveAllUpdates.
  • Front-end: shared testRender helper + extracted cloud sign-in dialog. Edge functions: shared _shared/paths.ts + S3 upload-verify. Launchers: shared process-tree / automation-ready helpers.

Verification: C# required filter 442/442, vitest + deno green. One item worth a look: ReceiveAllUpdates intentionally keeps its original which-books predicate (skips checked-out-here), which differs from GetUpdatesAvailableCount (excludes locked-by-anyone) — documented inline, not unified.

JohnThomson added a commit that referenced this pull request Jul 16, 2026
All 3 /simplify batches done + committed (6 logical-chunk commits); C# 442/442,
vitest/deno green. Live e2e blocked by environment (WebView2 about:blank under
the machine's memory pressure, not a regression). PR #8052 regenerated locally +
byte-identical, force-push held for John's explicit OK. Flags recorded:
master-merge/launcher reconciliation, live-e2e + launcher smoke for John's env,
and the ReceiveAllUpdates/GetUpdatesAvailableCount predicate drift.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
JohnThomson added a commit that referenced this pull request Jul 16, 2026
…PASS

With memory freed (49% free), e2e-1 + e2e-2 both pass through the live
Supabase+MinIO stack, live-verifying R1/R2 and ReceiveAllUpdates. Earlier
about:blank failures were the documented memory-pressure flake. PR #8052
regenerated + force-pushed byte-identical; Greptile re-triggered.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@JohnThomson
JohnThomson force-pushed the cloud-tc-for-review branch from 98bc352 to 9de218b Compare July 16, 2026 22:27
@JohnThomson

Copy link
Copy Markdown
Contributor Author

@greptile-apps review

(Claude Code, Opus 4.8) Regenerated after merging current origin/master into the branch and applying the remaining deferred efficiency/refactor items. Byte-identical to cloud-collections (the regen-rebuild identity check passes).

Backend efficiency (no behavior change):

  • E2 — cache the collection-scoped S3 download credentials and reuse them across books, instead of re-fetching per book (big win during a join).
  • E8 — the history panel now fetches only new events via the get_changes cursor and merges, instead of re-fetching the whole log on every open.
  • E9 — collection-file group download fetches a per-file manifest (get_collection_file_manifest) and downloads only the files whose content changed, each pinned to its committed S3 version — consistent snapshot, no redundant re-downloads.
  • E7 — the per-poll "self-healing" rename scan builds the instanceId → local-folder index once per poll and reuses it, so it is O(missing + local) instead of O(missing × local) disk stats.
  • E3 — idle polls (no new events, cursor unchanged) skip the full repo-cache Save + index rebuild; the group-file check and the self-healing download pass still run every poll.

Behavior changes (intentional, decided with John):

  • Cloud login now persists (DpapiCloudTokenStore) so a real sign-in survives a Bloom restart.
  • The "Updates Available (N)" count now includes books currently checked out by others (fetching their latest committed version is still useful).

Test-only refactors (no production change):

  • Shared CloudTestHarness fixture builder across the cloud unit tests.
  • Shared front-end hooks: useWatchApiDataWithReload and getApiDataOnce (dedup of the reload/cached-promise idioms in sharingApi/teamCollectionApi).

Verification: cloud/TC C# filter 453/453; front-end consumer vitest 38/38; pgTAP/deno green earlier this cycle; live e2e-1/e2e-2 previously passed.

JohnThomson added a commit that referenced this pull request Jul 16, 2026
Records the origin/master merge (10 commits, zero conflicts, merge not rebase
because cloud-collections is shared), the cloud-collections push, and the
cloud-tc-for-review regeneration (0 unmatched, byte-identical, force-pushed,
Greptile re-triggered).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
JohnThomson added a commit that referenced this pull request Jul 20, 2026
…kers

Documentation-only clarity pass over the Cloud Team Collections design docs,
prompted by review questions. No behavior change (comments and docs only).

- CONTRACTS.md: group the Postgres-RPC and edge-function sections under a new
  "Cloud functions" heading with a short primer on the difference (RPCs run in
  the database; edge functions are separate programs that alone hold the AWS
  creds), for readers who don't work in the DB backend.
- SCHEMA.md (new): a mermaid ER diagram of the whole tc schema, with reviewer
  notes (soft references; version_files holds only the current manifest; the
  ephemeral *_transactions tables) and instructions for keeping it in sync.
- config.toml: a DEV-ONLY banner on the [auth] block -- the whole file configures
  the local supabase stack and is never read by a hosted project; production auth
  is Firebase third-party.
- 20260706000001 (schema): a pointer to SCHEMA.md + "update the diagram when you
  change the schema" guidance.
- 20260706000004 (checkin txns): a DEV-ONLY caveat at the *_tx `authenticated`
  grants pointing to GOING-LIVE 2.4 (lock these down before production).
- REVIEW-PLAN.md (new): a prioritized human-review plan for PR #8052.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@JohnThomson
JohnThomson force-pushed the cloud-tc-for-review branch from 9de218b to e3b5a26 Compare July 20, 2026 13:52
@JohnThomson

Copy link
Copy Markdown
Contributor Author

@greptile-apps review

(Claude Code, Opus 4.8) Regenerated after a documentation + server-hardening pass from review discussion. Byte-identical to cloud-collections.

Docs / clarity (no behavior change):

  • CONTRACTS.md: a new "Cloud functions" section groups the Postgres-RPC and edge-function tables under a short primer on the difference (RPCs run in the database; edge functions are separate programs that alone hold the AWS creds) — for reviewers who don't work in the DB backend.
  • SCHEMA.md (new): a mermaid ER diagram of the whole tc schema + reviewer notes (soft references; version_files holds only the current manifest; ephemeral *_transactions).
  • Dev-only markers: a banner on config.toml's [auth] (whole file is local-stack; prod auth is Firebase third-party), and a caveat at the internal *_tx authenticated grants pointing to the "lock down before production" go-live item.
  • REVIEW-PLAN.md (new): a prioritized human-review plan for this PR.

Server hardening (new migrations + edge function, all tested):

  • Last-admin race: the last-admin guard now locks the collection row before counting, so two concurrent admin removals can't both slip through to zero admins (20260717000001).
  • Admin recovery: tc.support_set_admin(collection_id, email) — a service-role-only recovery tool (not callable by authenticated) for when a collection loses its only reachable admin (20260717000002); runbook in GOING-LIVE.
  • Orphaned-upload sweep: closes a real gap — a check-in that uploads to S3 but fails to commit leaves the committed version demoted to "noncurrent" (which the 7-day lifecycle rule would eventually delete) while the garbage upload sits as "current". tc.list_stale_upload_garbage() (20260717000003) is a reference-aware worklist (excludes files a live transaction is still uploading), and the new sweep-stale-uploads edge function deletes only versions newer than the committed one. GOING-LIVE documents scheduling it ~daily with the service-role key.

Verification: pgTAP 102/102; sweep Deno unit tests 4/4; deno check clean; C# cloud/TC filter unchanged (no C# changes this pass).

Note: the packaging branch is built on the same master commit as before (origin/master has moved ~4 commits since our last full merge); those are unrelated to this feature and will fold in at the next master merge.

@JohnThomson
JohnThomson force-pushed the cloud-tc-for-review branch from e3b5a26 to 305efe8 Compare July 20, 2026 18:07
@greptile-apps

greptile-apps Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Too many files changed for review. (252 files found, 100 file limit)

Bypass the limit by tagging @greptile-apps to review.

@JohnThomson
JohnThomson force-pushed the cloud-tc-for-review branch 6 times, most recently from ff19330 to 3d9068a Compare July 21, 2026 16:57
JohnThomson and others added 9 commits July 21, 2026 12:06
Part of the Cloud Team Collections feature (S3 + Supabase backed Team
Collections). This branch is a regenerable review-grained packaging of the
cloud-collections working branch; only the final tree is test-verified (see
Design/CloudTeamCollections/orchestration/SQUASH-PLAN.md).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Part of the Cloud Team Collections feature (S3 + Supabase backed Team
Collections). This branch is a regenerable review-grained packaging of the
cloud-collections working branch; only the final tree is test-verified (see
Design/CloudTeamCollections/orchestration/SQUASH-PLAN.md).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…transactions

Part of the Cloud Team Collections feature (S3 + Supabase backed Team
Collections). This branch is a regenerable review-grained packaging of the
cloud-collections working branch; only the final tree is test-verified (see
Design/CloudTeamCollections/orchestration/SQUASH-PLAN.md).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Part of the Cloud Team Collections feature (S3 + Supabase backed Team
Collections). This branch is a regenerable review-grained packaging of the
cloud-collections working branch; only the final tree is test-verified (see
Design/CloudTeamCollections/orchestration/SQUASH-PLAN.md).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…DK v4)

Part of the Cloud Team Collections feature (S3 + Supabase backed Team
Collections). This branch is a regenerable review-grained packaging of the
cloud-collections working branch; only the final tree is test-verified (see
Design/CloudTeamCollections/orchestration/SQUASH-PLAN.md).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…flow, background downloads, account-switch handling

Part of the Cloud Team Collections feature (S3 + Supabase backed Team
Collections). This branch is a regenerable review-grained packaging of the
cloud-collections working branch; only the final tree is test-verified (see
Design/CloudTeamCollections/orchestration/SQUASH-PLAN.md).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… book-list merge

Part of the Cloud Team Collections feature (S3 + Supabase backed Team
Collections). This branch is a regenerable review-grained packaging of the
cloud-collections working branch; only the final tree is test-verified (see
Design/CloudTeamCollections/orchestration/SQUASH-PLAN.md).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…oad placeholders

Part of the Cloud Team Collections feature (S3 + Supabase backed Team
Collections). This branch is a regenerable review-grained packaging of the
cloud-collections working branch; only the final tree is test-verified (see
Design/CloudTeamCollections/orchestration/SQUASH-PLAN.md).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Part of the Cloud Team Collections feature (S3 + Supabase backed Team
Collections). This branch is a regenerable review-grained packaging of the
cloud-collections working branch; only the final tree is test-verified (see
Design/CloudTeamCollections/orchestration/SQUASH-PLAN.md).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@JohnThomson
JohnThomson force-pushed the cloud-tc-for-review branch from 3d9068a to ad1b15d Compare July 21, 2026 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants